Rework the theme selector ahead of the design refresh by enf0rc3 · Pull Request #3259 · OctopusDeploy/docs

enf0rc3 · 2026-07-28T04:40:24Z

Rewrites the theme switcher so it is a solid base for the design refresh. A 1:1 port of the existing behaviour, with the bugs fixed and the moving parts in idiomatic Astro places.

Bugs fixed

The two switchers desynced. Each instance cached isDarkMode at construction, so toggling the mobile one left the header one stale. Resize past 1130px and it showed the wrong position; clicking it then set data-theme to the value it already had.
The knob animated on every page load. Slider position came from a JS-applied .dark-mode class, so in dark mode it painted on the light side and slid across after hydration. Now driven by html[data-theme], correct on the first frame.
Keyboard users tabbed to a non-widget. tabindex="-1" on the input plus tabindex="0" on the label meant the focusable element was a bare <label> — no role, name or state exposed to a screen reader. The real input[role=switch] is focusable again, the deprecated keyCode handler is gone, and the focus ring is drawn via input:focus-visible + label.
localStorage was unguarded. A throw in the head script (Safari private mode, cookie-blocked iframes) killed the IIFE and left the site permanently light regardless of OS preference.
Enter did not toggle the switch. A native checkbox ignores Enter, so it is bound explicitly and routed through click(), keeping the change listener the only writer of state. Space and Enter are both tested.

What changed

File
`src/lib/theme.ts`	new — attribute names, storage key and types in one place
`src/scripts/theme-switcher.ts`	new — the controller, bundled by Astro
`src/themes/octopus/components/ThemeScript.astro`	new — pre-paint inline script, values via `define:vars` from `theme.ts`
`src/components/ThemeSwitcher.astro`	new — one switcher component, takes an `id`
`tests/theme.spec.ts`	new — 9 tests
`public/docs/js/modules/theme-switcher.js`	deleted

The controller moved from public/ into src/scripts/, so Astro bundles, minifies and dedupes it rather than shipping an unhashed public file loaded via is:inline.
The pre-paint script moved out of the HEADER_SCRIPTS string in config.ts into ThemeScript.astro, rendered first in <head> — ahead of the stylesheet links, so the parser is not waiting on a CSS download to run it. It stays is:inline deliberately; it must not be bundled or deferred.
Switcher markup deduplicated into ThemeSwitcher.astro, used by both Header.astro and MobileMenu.astro.
astro:after-swap handler so the switcher keeps working if <ClientRouter /> is ever added.
OS preference changes are followed until the user picks a side; other tabs sync via the storage event.
Removed the dead <body data-theme="dark">, and the DOMContentLoaded wrapper the old script did not need.
Accessible name is now "Dark mode" — "Toggle theme between light and dark mode, switch, on" read badly.

We tried adding color-scheme so native scrollbars would follow the theme, but our existing text-colour transitions made it look wrong, so it is not included.

Testing

tests/theme.spec.ts — 9 tests, all passing: OS preference followed in both schemes, explicit choice beating the OS preference and surviving navigation, both switchers staying in sync, Space and Enter each toggling both ways, fallback when localStorage throws, data-theme present with scripting disabled, and a regression guard that the knob position is attribute-driven rather than script-driven.

pnpm build passes (2697 pages).

I have also manually tested the above bugs and can confirm they are resolved, i found one trivial thing that was fixed being pressing enter on the toggle when tabbing through ui

🤖 Generated with Claude Code

team-marketing-branch-protections · 2026-07-28T04:58:09Z

Pull request environment is available at https://stoctodocspr3259.z22.web.core.windows.net.

You can view the ephemeral environment status in Octopus Deploy.

This environment will be automatically deprovisioned when the pull request is closed, or after 7 days of inactivity.

borland · 2026-07-28T21:32:17Z

I don't profess to understand the nitty-gritty detail of css and root selectors vs other things, but at a high level it all seems sound and reasonable.

I noticed though on the staging site, when switching between light and dark mode, the color changes on the header items now "pulse" in a way that they didn't before.

To experience this, go to this page: https://octopus.com/docs/infrastructure/accounts/openid-connect on the main site, and toggle between light and dark a few times quickly.

Then do the same on the staging site: https://stoctodocspr3259.z22.web.core.windows.net/docs/infrastructure/accounts/openid-connect and note how all the header text such as "OpenID Connect" animates differently.

While personally I think it's kind of weird looking, It isn't unacceptable -- we could choose to accept the new color changes. But if we can fix them instead, we should

enf0rc3 · 2026-07-28T23:15:40Z

Thanks for finding that bug @borland So it turns out it was the addition of using "color-scheme" because of pre-existing "transtions" that were defined on main.

Super weird to have transitions there, but it may be there for a reason, so i reverted the scroll bar back to has it was to not use color-scheme

Move theme handling onto a single contract in src/lib/theme.ts, shared by the pre-paint inline script, the client controller and the stylesheets. <html> now always carries data-theme="light" or "dark" (never absent, never "system"), so incoming design token stylesheets can key off the attribute with no :root fallback. data-theme-preference records the raw choice, including "system", ready for a three-position control. Fixes along the way: - The two switchers each cached their own state, so toggling one left the other stale. Both now read the same attribute. - The knob position came from a JS-applied class, so it painted on the light side and slid across on every load. It is now driven by html[data-theme]. - tabindex="-1" on the input plus tabindex="0" on the label made the focusable element a bare <label> with no role, name or state. The input is focusable again and the deprecated keyCode handler is gone. - A localStorage throw in the head script left the site permanently light. - color-scheme is set per theme, so native controls and scrollbars follow. - OS preference changes are followed until the user picks a side; other tabs stay in sync via the storage event. The controller moves from public/ into src/ so Astro bundles, minifies and dedupes it, and an astro:after-swap handler keeps it working if <ClientRouter /> is ever added. Adds tests/theme.spec.ts (7 tests) and PLAYWRIGHT_PORT so the suite can run beside a dev server. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Moving to a native checkbox meant only Space toggled it - a checkbox ignores Enter. The switch role treats Enter as an optional second activation key and the control supported it before, so bind it explicitly on the input. Tests now press each key twice to check it toggles both ways. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

- Route the Enter handler through box.click() so the change listener stays the only writer of theme state, instead of flipping box.checked separately. - Scope the dark slider rule to .theme-switcher so a future .switch-slider elsewhere does not inherit it. - Use auto-retrying toHaveAttribute assertions in the theme spec rather than one-shot getAttribute reads. - Document that theme:change does not fire at boot, and that the src/pages/report/*.astro pages render their own <html> and sit outside the data-theme contract. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

#3261 added the token CSS, which defines its custom properties only under [data-theme='light'] and [data-theme='dark'] with nothing at :root, and added a server-rendered data-theme="light" on <html> so the attribute is present even without scripting. Both are kept: ThemeScript.astro resolves the real value before first paint on top of that default. - Point the HtmlHead token comment at ThemeScript.astro rather than HEADER_SCRIPTS, which no longer holds the theme script. - Spell out in theme.ts why the attribute has to be unconditional. - Add a javaScriptEnabled: false test so the server-rendered default cannot be dropped again without a failure. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

color-scheme was added by this branch as polish so native scrollbars would follow the theme. It is not a normal paint property: it changes how colours are resolved document-wide, and flipping it in the same style recalculation as the themed colours makes Chromium restart every descendant `color` transition on each frame. The effect was two visible transitions instead of one. Headings crawled about 10% of the way over 300ms while body's transition was in flight, then ran their real 300ms fade once body settled - roughly 640ms end to end. The crossfade is deliberate: 23f3a01 added it with a `prefers-reduced-motion: reduce` guard on every rule. Removing these two declarations restores it. Measured on a real build, heading colour now completes in one ramp at 291ms, matching the pre-branch timing. Scrollbars go back to the browser default in dark mode, as before this branch. `scrollbar-color` would theme them without disturbing colour resolution, and measured clean, but that is a separate change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The PLAYWRIGHT_PORT override was only there so I could run the suite while a dev server held 3000. It is not needed by this change, and `webServer.port` alone cannot move the server - `astro preview` takes its port from server.port in astro.config.mjs, so it needed a --port flag as well, which is more explaining than the convenience is worth. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

enf0rc3 force-pushed the wl/theme-selector-rework branch from 84e1d85 to 8f97202 Compare July 28, 2026 04:41

enf0rc3 mentioned this pull request Jul 28, 2026

Consume design tokens as CSS custom properties #3261

Merged

enf0rc3 marked this pull request as ready for review July 28, 2026 05:29

enf0rc3 force-pushed the wl/theme-selector-rework branch from 3d27ca5 to cd273a3 Compare July 28, 2026 20:35

enf0rc3 commented Jul 28, 2026

View reviewed changes

Comment thread playwright.config.ts Outdated

borland reviewed Jul 28, 2026

View reviewed changes

Comment thread playwright.config.ts Outdated

enf0rc3 requested a review from borland July 28, 2026 23:26

enf0rc3 and others added 7 commits July 29, 2026 11:44

Drop two explanatory comments from the theme CSS

402bafa

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

enf0rc3 force-pushed the wl/theme-selector-rework branch from 5046d6f to 0892d9f Compare July 28, 2026 23:48

borland approved these changes Jul 29, 2026

View reviewed changes

enf0rc3 merged commit 538f3fc into main Jul 29, 2026
7 checks passed

enf0rc3 deleted the wl/theme-selector-rework branch July 29, 2026 01:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Rework the theme selector ahead of the design refresh - #3259

Rework the theme selector ahead of the design refresh#3259
enf0rc3 merged 7 commits into
mainfrom
wl/theme-selector-rework

enf0rc3 commented Jul 28, 2026 •

edited

Loading

Uh oh!

team-marketing-branch-protections commented Jul 28, 2026

Uh oh!

Uh oh!

Uh oh!

borland commented Jul 28, 2026

Uh oh!

enf0rc3 commented Jul 28, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

enf0rc3 commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Bugs fixed

What changed

Testing

Uh oh!

team-marketing-branch-protections commented Jul 28, 2026

Uh oh!

Uh oh!

Uh oh!

borland commented Jul 28, 2026

Uh oh!

enf0rc3 commented Jul 28, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

enf0rc3 commented Jul 28, 2026 •

edited

Loading